fix(env): stop an unreadable environment setting from picking a wrong default - #1880

… default

atoi and atol answer 0 for text they cannot read, and 0 is a real setting at
three places in this project. So a typo, a trailing unit such as "30s", or a
stray space silently chose a value nobody asked for, and nothing on screen
said the setting had been dropped.

  src/mcp/index_supervisor.c  CBM_INDEX_WORKER_TIMEOUT_S
  src/cli/hook_augment.c      CBM_HOOK_DEADLINE_MS
  src/mcp/mcp.c               CBM_INDEX_MAX_RESTARTS

CBM_HOOK_DEADLINE_MS was the worst of the three. atoi answered 0, 0 is below
HA_DEADLINE_MIN_MS, and the clamp then handed back 50 ms -- the SHORTEST
deadline the setting allows, for a setting whose only purpose is to give the
hook more room. The comment above that function records a hunt for hook runs
that never finished, 0 of 24 real sessions, which is the exact symptom a
silently-shortened deadline produces.

CBM_INDEX_MAX_RESTARTS lost twice. A typo kept the default of 100, and
CBM_INDEX_MAX_RESTARTS=0 -- which reads as "do not restart" to anybody who
sets it -- also kept 100. The setting did the opposite of the request.

CBM_INDEX_WORKER_TIMEOUT_S fell through to the 15-minute default, so a test
set to give up after 30 seconds hung for 15 minutes with nothing to explain
why.

The fix adds one helper rather than three copies of the same check:

  bool cbm_env_long(const char *name, long *out);

It answers true only when the variable is set, is not empty, and reads
cleanly from its first character to its last. It holds no policy -- no
minimum, no maximum, no default -- because the three sites disagree on all
three, and a helper that guessed would be wrong at two of them. The shape is
the one src/main.c:1104 already uses: an end pointer, errno, and a check that
nothing was left over. It also refuses a leading blank, which strtol would
otherwise step over, so " 5" is a slip rather than the number 5.

Each site keeps its own rule:

  worker timeout    an unreadable value keeps the 15-minute default AND logs
                    the value it dropped
  restart cap       0 now means no restarts; an unreadable value keeps 100
                    AND logs the value it dropped
  hook deadline     an unreadable value now yields HA_DEADLINE_DEFAULT_MS,
                    not the floor. This one stays silent on purpose: the file
                    includes no log header and writes no stderr, because its
                    output is hook protocol.

The restart-cap parse was lifted out of a very large function into a named
index_restart_cap(), so it can be read and reached on its own.

Four tests come with the change. The two that pin the user-visible behaviour
were seen failing before the fix:

  FAIL tests/test_cli.c:402: ms == 50, expected HOOK_DEADLINE_DEFAULT == 2000
    (with "unreadable value \"abc\" gave 50 ms" printed above it)
  FAIL tests/test_cli.c:434: cbm_index_restart_cap_for_testing() == 100,
    expected 0 == 0

After the fix, TEST_SUITES="platform cli mcp" reports 518 passed, 2 failed.
The full suite reports 7635 passed, 2 failed. Both failures are in
tests/test_cli.c (lines 1826 and 6802), print "error: one or more agent
cleanup operations failed", and reproduce on a clean tree without this
change -- they depend on the coding agents installed on the machine.
make -f Makefile.cbm lint-ci passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>